From 56cd8dbcd214bab61f717ad7484d8014c50b25ef Mon Sep 17 00:00:00 2001 From: robertlipe Date: Sun, 14 Dec 2014 00:13:39 +0000 Subject: [PATCH] Try to use xasprintf less often when we're putting it into a QString anyway. --- gpsbabel/README.contrib | 12 +++++++++ gpsbabel/bushnell.cc | 7 +++--- gpsbabel/exif.cc | 7 +++--- gpsbabel/gbfile.cc | 13 ++++++++++ gpsbabel/gbfile.h | 1 + gpsbabel/magproto.cc | 8 +++--- gpsbabel/text.cc | 4 +-- gpsbabel/tpo.cc | 56 ++++++++++++++++------------------------- 8 files changed, 60 insertions(+), 48 deletions(-) diff --git a/gpsbabel/README.contrib b/gpsbabel/README.contrib index b195b7e60..3f865f2b0 100644 --- a/gpsbabel/README.contrib +++ b/gpsbabel/README.contrib @@ -12,6 +12,18 @@ functions. You may find format_skeleton.c and filter_skeleton.c in the source tree to be helpful examples. Just add meat! +Prefer Qt objects/classes to ISO C/POSIX services. QStrings are reference +counted, implicitly shared, and have a robust supporting library. QDateTime +supports sub-second time and a range of dates far beyond 1970->2038 and are +much more pleasant to work with than ctime/mktime/struct tm. "But I see +strcpy, sprintf, and struct tm and such in the code!" It's true; GPSBabel +is a tenured project of well over ten years. We have code that predates +our move to C++/Qt that isn't well tested or has a low payoff to modernize +and uses old constructs. Our actively maintained/strategic formats like +GPX and KML tend to be better examples of modern programming, using things +like XmlStreamWriter instead of fprintf and are generally better models to +follow. + Compilers complain for a reason. Code shouldn't emit warnings. The entire world doesn't run . We regularly test this code on diff --git a/gpsbabel/bushnell.cc b/gpsbabel/bushnell.cc index a131cea7a..8fa844db9 100644 --- a/gpsbabel/bushnell.cc +++ b/gpsbabel/bushnell.cc @@ -231,8 +231,10 @@ bushnell_write_one(const Waypoint* wpt) char padding[2] = {0, 0}; gbfile* file_out; static int wpt_count; - char* fname; - xasprintf(&fname, "%s-%d.wpt", ofname, wpt_count++); + QString fname(ofname); + fname += "-"; + fname += QString::number(wpt_count++); + fname += ".wpt"; file_out = gbfopen_le(fname, "wb", MYNAME); gbfputint32(round(wpt->latitude * 10000000), file_out); @@ -247,7 +249,6 @@ bushnell_write_one(const Waypoint* wpt) // two padding bytes follow name. gbfwrite(padding, sizeof(padding), 1, file_out); - xfree(fname); gbfclose(file_out); } diff --git a/gpsbabel/exif.cc b/gpsbabel/exif.cc index 8325d2b5c..7c0fee3ff 100644 --- a/gpsbabel/exif.cc +++ b/gpsbabel/exif.cc @@ -1370,7 +1370,6 @@ static void exif_wr_init(const char* fname) { uint16_t soi; - char* tmpname; exif_success = 0; exif_fout_name = xstrdup(fname); @@ -1392,9 +1391,9 @@ exif_wr_init(const char* fname) fatal(MYNAME ": No valid timestamp found in picture!\n"); } abort(); - xasprintf(&tmpname, "%s.jpg", fname); - fout = gbfopen_be(tmpname, "wb", MYNAME); - xfree(tmpname); + QString filename(fname); + filename += ".jpg"; + fout = gbfopen_be(filename, "wb", MYNAME); } static void diff --git a/gpsbabel/gbfile.cc b/gpsbabel/gbfile.cc index fcb05e3c5..a9b8a803e 100644 --- a/gpsbabel/gbfile.cc +++ b/gpsbabel/gbfile.cc @@ -695,6 +695,19 @@ gbfread(void* buf, const gbsize_t size, const gbsize_t members, gbfile* file) return file->fileread(buf, size, members, file); } +// This probably makes an unnecessary alloc/copy, but keeps the above (kinda +// goofy) calling signature. +gbsize_t +gbfread(QString& buf, const gbsize_t size, + const gbsize_t members, gbfile* file) +{ + // xcalloc() ensures the buf is zero terminated, so it's a proper c string. + char* tmp = static_cast(xcalloc(members, size)); + gbsize_t retval = gbfread(tmp, size, members, file); + buf = QString(tmp); + return retval; +} + /* * gbvfprintf: (as vfprintf) */ diff --git a/gpsbabel/gbfile.h b/gpsbabel/gbfile.h index 0a69a0bc5..1c2c6e848 100644 --- a/gpsbabel/gbfile.h +++ b/gpsbabel/gbfile.h @@ -92,6 +92,7 @@ gbfile* gbfopen_be(const QString filename, const char* mode, const char* module) void gbfclose(gbfile* file); gbsize_t gbfread(void* buf, const gbsize_t size, const gbsize_t members, gbfile* file); +gbsize_t gbfread(QString& buf, const gbsize_t size, const gbsize_t members, gbfile* file); int gbfgetc(gbfile* file); QString gbfgets(char* buf, int len, gbfile* file); diff --git a/gpsbabel/magproto.cc b/gpsbabel/magproto.cc index 2c559de73..2c9e11be5 100644 --- a/gpsbabel/magproto.cc +++ b/gpsbabel/magproto.cc @@ -1044,7 +1044,6 @@ mag_rteparse(char* rtemsg) char* currtemsg; static mag_rte_head* mag_rte_head; char* p; - char* rte_name = NULL; #if 0 sscanf(rtemsg,"$PMGNRTE,%d,%d,%c,%d%n", @@ -1054,6 +1053,7 @@ mag_rteparse(char* rtemsg) &frags,&frag,xbuf,&rtenum,&n); /* Explorist has a route name here */ + QString rte_name; if (explorist) { char* ca, *ce; @@ -1064,7 +1064,8 @@ mag_rteparse(char* rtemsg) is_fatal(ce == NULL, MYNAME ": Incorrectly formatted route line '%s'", rtemsg); if (ca == ce) { - xasprintf(&rte_name, "Route%d", rtenum); + rte_name = "Route"; + rte_name += QString::number(rtenum); } else { rte_name = xstrndup(ca, ce - ca); } @@ -1164,9 +1165,6 @@ mag_rteparse(char* rtemsg) } xfree(mag_rte_head); } - if (rte_name) { - xfree(rte_name); - } } QString diff --git a/gpsbabel/text.cc b/gpsbabel/text.cc index 847f20f77..8cc01478c 100644 --- a/gpsbabel/text.cc +++ b/gpsbabel/text.cc @@ -107,8 +107,8 @@ text_disp(const Waypoint* wpt) waypoint_count++; if (split_output) { - char* thisfname; - xasprintf(&thisfname, "%s%d", output_name, waypoint_count); + QString thisfname(output_name); + thisfname += QString::number(waypoint_count); file_out = gbfopen(thisfname, "w", MYNAME); } diff --git a/gpsbabel/tpo.cc b/gpsbabel/tpo.cc index c9a293e60..13816f67c 100644 --- a/gpsbabel/tpo.cc +++ b/gpsbabel/tpo.cc @@ -612,7 +612,6 @@ void tpo_process_tracks(void) unsigned int line_type; unsigned int track_style; unsigned int name_length; - char* track_name = NULL; unsigned int track_byte_count; int llvalid; unsigned char* buf; @@ -642,17 +641,14 @@ void tpo_process_tracks(void) //UNKNOWN DATA LENGTH name_length = tpo_read_int(); - + QString track_name; if (name_length) { - track_name = (char*) xmalloc(name_length+1); - track_name[0] = '\0'; gbfread(track_name, 1, name_length, tpo_file_in); - track_name[name_length] = '\0'; // Terminator } else { // Assign a generic track name - xasprintf(&track_name, "TRK %d", ii+1); + track_name = "TRK "; + track_name += QString::number(ii + 1); } track_temp->rte_name = track_name; - xfree(track_name); // RGB line_color expressed for html=rrggbb and kml=bbggrr - not assigned before 2012 sprintf(rgb,"%02x%02x%02x",styles[track_style].color[0],styles[track_style].color[1],styles[track_style].color[2]); @@ -671,15 +667,16 @@ void tpo_process_tracks(void) track_temp->line_width = styles[track_style].wide; if (DEBUG) printf("Track Name: %s, ?Type?: %d, Style Name: %s, Width: %d, Dashed: %d, Color: #%s\n", - track_name, line_type, styles[track_style].name, styles[track_style].wide, styles[track_style].dash,rgb); + qPrintable(track_name), line_type, styles[track_style].name, styles[track_style].wide, styles[track_style].dash,rgb); // Track description - // track_temp->rte_desc = NULL; // pre-2012 default, next line from SRE saves track style as track description - xasprintf(&track_temp->rte_desc, "Style=%s, Width=%d, Dashed=%d, Color=#%s", - styles[track_style].name, styles[track_style].wide, styles[track_style].dash, rgb); + track_temp->rte_desc = + QString().sprintf("Style=%s, Width=%d, Dashed=%d, Color=#%s", + styles[track_style].name, styles[track_style].wide, + styles[track_style].dash, rgb); // Route number - track_temp->rte_num = ii+1; + track_temp->rte_num = ii + 1; //UNKNOWN DATA LENGTH track_byte_count = tpo_read_int(); @@ -806,10 +803,6 @@ void tpo_process_tracks(void) Waypoint** tpo_wp_index; unsigned int tpo_index_ptr; - - - - // Waypoint decoder for version 3.x files. // void tpo_process_waypoints(void) @@ -844,7 +837,6 @@ void tpo_process_waypoints(void) Waypoint* waypoint_temp; Waypoint* waypoint_temp2; unsigned int name_length; - char* waypoint_name; int lat; int lon; unsigned int altitude; @@ -858,16 +850,13 @@ void tpo_process_waypoints(void) //UNKNOWN DATA LENGTH // Fetch name length name_length = tpo_read_int(); -//printf("\nName Length: %d\n", name_length); + QString waypoint_name; if (name_length) { - waypoint_name = (char*) xmalloc(name_length+1); - waypoint_name[0] = '\0'; gbfread(waypoint_name, 1, name_length, tpo_file_in); - waypoint_name[name_length] = '\0'; // Terminator } else { // Assign a generic waypoint name - xasprintf(&waypoint_name, "WPT %d", ii+1); + waypoint_name = "WPT "; + waypoint_name += QString::number(ii + 1); } -//printf("\tWaypoint Name: %s\n", waypoint_name); //UNKNOWN DATA LENGTH (void)tpo_read_int(); @@ -880,7 +869,6 @@ void tpo_process_waypoints(void) // Assign the waypoint name waypoint_temp->shortname = waypoint_name; - xfree(waypoint_name); // Grab the altitude in meters altitude = gbfgetint32(tpo_file_in); @@ -985,7 +973,7 @@ void tpo_process_map_notes(void) waypoint_temp = tpo_convert_ll(lat, lon); // Assign a generic waypoint name - waypoint_temp->shortname = QString().sprintf("NOTE %d", ii+1); + waypoint_temp->shortname = QString().sprintf("NOTE %d", ii + 1); //UNKNOWN DATA LENGTH (void)tpo_read_int(); @@ -1121,7 +1109,7 @@ void tpo_process_symbols(void) waypoint_temp = tpo_convert_ll(lat, lon); // Assign a generic waypoint name - waypoint_temp->shortname = QString().sprintf("SYM %d", ii+1); + waypoint_temp->shortname = QString().sprintf("SYM %d", ii + 1); // waypoint_temp->description = NULL; // waypoint_temp->notes = NULL; @@ -1183,7 +1171,7 @@ void tpo_process_text_labels(void) waypoint_temp = tpo_convert_ll(lat, lon); // Assign a generic waypoint name - waypoint_temp->shortname = QString().sprintf("TXT %d", ii+1); + waypoint_temp->shortname = QString().sprintf("TXT %d", ii + 1); for (jj = 0; jj < 16; jj++) { //UNKNOWN DATA LENGTH @@ -1252,7 +1240,6 @@ void tpo_process_routes(void) // for (ii = 0; ii < route_count; ii++) { unsigned int name_length = 0; - char* route_name; unsigned int jj; unsigned int waypoint_cnt; route_head* route_temp; @@ -1271,16 +1258,17 @@ void tpo_process_routes(void) //UNKNOWN DATA LENGTH // Fetch name length name_length = tpo_read_int(); + QString route_name; if (name_length) { - route_name = (char*) xmalloc(name_length+1); - route_name[0] = '\0'; + //route_name = (char*) xmalloc(name_length+1); + //route_name[0] = '\0'; gbfread(route_name, 1, name_length, tpo_file_in); - route_name[name_length] = '\0'; // Terminator + //route_name[name_length] = '\0'; // Terminator } else { // Assign a generic route name - xasprintf(&route_name, "RTE %d", ii+1); + route_name = "RTE "; + route_name += QString::number(ii + 1); } route_temp->rte_name = route_name; - xfree(route_name); //printf("Route Name: %s\n", route_name); @@ -1290,7 +1278,7 @@ void tpo_process_routes(void) // gbfgetc(tpo_file_in); // route_temp->rte_desc = NULL; - route_temp->rte_num = ii+1; + route_temp->rte_num = ii + 1; // Fetch the number of waypoints in this route. 8/16/32-bit // value. -- 2.30.2